Skip to content

Wait for dock removed#37

Merged
podviaznikov merged 26 commits intomasterfrom
wait-for-dock-removed
Dec 7, 2015
Merged

Wait for dock removed#37
podviaznikov merged 26 commits intomasterfrom
wait-for-dock-removed

Conversation

@anandkumarpatel
Copy link
Copy Markdown
Contributor

  • change on-dock-removed to event dock.removed
  • ensure dock is out of swarm rotation before emitting dock-removed

review

related PR

CodeNow/devops-scripts#224

@podviaznikov
Copy link
Copy Markdown
Member

dock-removed should be dock.removed according to your own convention

@anandkumarpatel
Copy link
Copy Markdown
Contributor Author

yep your right, still working on this one

Comment thread lib/models/worker-server.js Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this one is new queue: use your command patter naming convention please

Comment thread lib/models/events.js Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

log error

@cflynn07
Copy link
Copy Markdown
Contributor

cflynn07 commented Dec 7, 2015

LGTM

Comment thread test/unit/models/consul.js Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.exist(Error) is not a thing. exist

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and your test says TaskError not Error

@podviaznikov
Copy link
Copy Markdown
Member

works on beta!

podviaznikov added a commit that referenced this pull request Dec 7, 2015
@podviaznikov podviaznikov merged commit ce7e802 into master Dec 7, 2015
@podviaznikov podviaznikov deleted the wait-for-dock-removed branch December 7, 2015 23:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants